Skip to content

Fix temp file leaks and correct return type annotations#185

Merged
absadiki merged 1 commit into
absadiki:mainfrom
sorlen008:fix/temp-file-leaks-and-types
Apr 20, 2026
Merged

Fix temp file leaks and correct return type annotations#185
absadiki merged 1 commit into
absadiki:mainfrom
sorlen008:fix/temp-file-leaks-and-types

Conversation

@sorlen008

Copy link
Copy Markdown
Contributor

Summary

Found a few bugs while reading through the codebase -- two temp file leaks, incorrect return type annotations, and some type() == str checks that should be isinstance().

Bug 1: Temp file leak in auto_sync()

auto_sync() creates temp files and appends .ass / .srt extensions to them (e.g. srtin = srtin_file.name + '.ass'). The finally block was deleting the extensionless base files (srtin_file.name, srtout_file.name) but never the actual .ass and .srt files that ffsubsync writes to. Every call to auto_sync() leaked two files in the system temp directory.

Fix: Added cleanup for srtin and srtout (the paths with extensions) in the finally block, with os.path.exists() guards since they may not have been created if an error happened early.

Bug 2: Temp file leak in WhisperAPIModel.transcribe()

convert_video_to_audio_ffmpeg() creates a temp audio file (e.g. /tmp/filename.mp3) but after transcription finishes, that file was never cleaned up. Repeated transcriptions would accumulate audio files in the temp directory.

Fix: Wrapped the transcription body in a try/finally that removes the converted audio file after use.

Bug 3: Wrong -> str return type on transcribe() in 6 models

The abstract base class correctly declares transcribe() -> SSAFile, and all implementations actually return SSAFile objects. But 6 concrete model files had -> str as the return type annotation:

  • whisper_model.py
  • whisper_api_model.py
  • whisper_timestamped_model.py
  • whisperX_model.py
  • faster_whisper_model.py
  • whispercpp_model.py

Fix: Changed all to -> SSAFile. Added the missing from pysubs2 import SSAFile to whisper_model.py (the others already had it).

Bug 4: type(model) == str instead of isinstance()

Three places in main.py used type(model) == str which won't match subclasses of str and is generally considered non-Pythonic.

Fix: Changed all three to isinstance(model, str).

Test plan

  • Verify auto_sync() no longer leaves .ass/.srt files in temp directory after use
  • Verify WhisperAPIModel.transcribe() cleans up the converted audio file
  • Confirm type checkers (mypy/pyright) no longer flag return type mismatches on transcribe()
  • Run existing test suite (python -m pytest tests/)

🤖 Generated with Claude Code

…annotations

- auto_sync() now cleans up the .ass and .srt temp files (previously only
  the extensionless NamedTemporaryFile bases were deleted)
- WhisperAPIModel.transcribe() now removes the converted audio file in a
  finally block so it doesn't leak on success or failure
- Changed transcribe() return type from -> str to -> SSAFile in all 6
  model files to match the actual return value and the abstract base class
- Replaced type(model) == str with isinstance(model, str) in 3 places

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@absadiki absadiki merged commit 5ed78a8 into absadiki:main Apr 20, 2026
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants